fix(users): Check key manager runtime feature flag when calling transfer key #10859
Conversation
|
Review changes with Changed Files
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10859 +/- ##
=======================================
Coverage ? 0
=======================================
Files ? 0
Lines ? 0
Branches ? 0
=======================================
Hits ? 0
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
To check if the dependencies are being enabled correctly or not, I've disabled I've added a piece of code in the With the old feature flag dependencies, with the above setup, the log outputs With the new ones, the log shows false, which is expected. |
|
These dependencies are fragile though. There is a very high chance that somebody would later use wrong dependency graph, which will revert back to the old behaviour. EDIT: Should not be a problem because in all the images we give out, |
… create functions
ThisIsMani
requested review from
ShankarSinghC,
racnan,
sai-harsha-vardhan and
tsdk02
tsdk02
left a comment
tsdk02
left a comment
There was a problem hiding this comment.
Minor nit, other than that, looks good to me!
| }) | ||
| } | ||
|
|
||
| pub fn is_encryption_service_enabled(&self) -> bool { |
There was a problem hiding this comment.
Instead of having feature flag based branching in a fn, we can have two definitions for v1 and v2 right
Type of Change
Description
We use
transfer_key_to_key_managerto transfer key to the encryption service. When that function was introduced in the user module, there was only compile time feature flag. Afterwards a runtime feature flag is also introduced which when disabled, the expectation is the encryption service is not supposed to be used.When this runtime flag is introduced, the user code was not updated to check if the runtime feature flag is enabled before calling the transfer function. Because of this, the code is trying to transfer the key to encryption service even when the runtime feature flag is set to false.
Ideally, this check should've been in transfer function itself since that is how all the other functions (
crypto_operation) functions are written.This PR adds check in
transfer_key_to_key_managerfunction which checks if the runtime feature flag for key manager is enabled or not.This was being done manually before, and was not added in user part of code, which caused some problems, so, I moved the check to the transfer function itself and callers doesn't have to check this manually from now on.
Additional Changes
Motivation and Context
Closes #4071
How did you test it?
Ran both Hyperswitch and Encryption Service locally. Turned on the
encryption_servicecompile time feature flag in Hyperswitch.Turned on the runtime feature flag, created a user, which creates merchant and setup TOTP, both merchant and user related operations are done in this flow along with encryption and decryption. Everything worked in this case.
Turned off the runtime feature flag and logged in with the old user, which also worked. - This was the main problem before.
Created a new user in this state (runtime flag turned off).
Turned on the runtime feature flag and logged in again with both users. Second user has fell back to local decryption since the key is not present in encryption service, everything worked as expected.
Checklist
cargo +nightly fmt --allcargo clippy